Allow settings to be updated and read. #2985
Conversation
anton-vlasenko
left a comment
anton-vlasenko
left a comment
There was a problem hiding this comment.
Please find my review notes below. Thank you.
| wp_set_current_user( self::$administrator ); | ||
| $user = get_user_by( 'id', self::$administrator ); | ||
| $user->add_cap( 'custom_rest_cap' ); |
There was a problem hiding this comment.
In my testing, this test didn't work because the user's capabilities were cached. Therefore, I had to reorder it like this to make the test work:
| wp_set_current_user( self::$administrator ); | |
| $user = get_user_by( 'id', self::$administrator ); | |
| $user->add_cap( 'custom_rest_cap' ); | |
| $user = get_user_by( 'id', self::$administrator ); | |
| $user->add_cap( 'custom_rest_cap' ); | |
| wp_set_current_user( self::$administrator ); |
| wp_set_current_user( self::$administrator ); | ||
| $user = get_user_by( 'id', self::$administrator ); | ||
| $user->add_cap( 'custom_rest_cap' ); |
There was a problem hiding this comment.
Same.
| wp_set_current_user( self::$administrator ); | |
| $user = get_user_by( 'id', self::$administrator ); | |
| $user->add_cap( 'custom_rest_cap' ); | |
| $user = get_user_by( 'id', self::$administrator ); | |
| $user->add_cap( 'custom_rest_cap' ); | |
| wp_set_current_user( self::$administrator ); |
| 'status' => rest_authorization_required_code(), | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
The WP_Test_REST_Settings_Controller::test_update_item_with_edit_permission_callbacks() test method implies that the setting can be updated even if the current user lacks the manage_options capability. However, the test fails because the code still checks for that capability.
I'm not sure what the intention is here: should a user be able to update a setting even without the manage_options capability, as long as the permission callback allows it?
| } | |
| } | |
| return true; |
| * @since 6.1.0 | ||
| * | ||
| * @param WP_REST_Request $request Full details about the request. | ||
| * @return true|WP_Error True if the request has access to update the item, WP_Error object otherwise. |
There was a problem hiding this comment.
Nitpick:
| * @return true|WP_Error True if the request has access to update the item, WP_Error object otherwise. | |
| * @return bool|WP_Error True if the request has access to update the item, WP_Error object otherwise. |
2 participants
Working on #48885. Decided to go a different direction than the ticket name. Use the existing endpoint but add some new callbacks.
To use this PR, here is the example I use to test.
This make a setting called
wibble, that will now be public to read and edit. You never do this, but it gives you an idea of how the code works.Trac ticket: https://core.trac.wordpress.org/ticket/48885
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.